Skip to content

Conversation

@mocenas
Copy link
Contributor

@mocenas mocenas commented Dec 1, 2025

Adding a tests for #50751 where KafkaCompanionResource got double-quoted channel name.
These tests are used in QE TS, but might also be useful here.

@mocenas
Copy link
Contributor Author

mocenas commented Dec 1, 2025

cc @radcortez @gsmet

@gsmet
Copy link
Member

gsmet commented Dec 1, 2025

@mocenas 👋 the test is failing and requires a fix or it's all fine?

@mocenas
Copy link
Contributor Author

mocenas commented Dec 1, 2025

@mocenas 👋 the test is failing and requires a fix or it's all fine?

That's weird it passes for me no problem, executing in using mvn -pl integration-tests/reactive-messaging-kafka clean verify -Dtest-containers -Dtest=TransmitterTest

What failure do you have with it?

@mocenas mocenas force-pushed the kafka_test_donation branch from f809237 to ad86b8e Compare December 1, 2025 13:07
@mocenas
Copy link
Contributor Author

mocenas commented Dec 1, 2025

I rebased the PR on up-to-date main.

@quarkus-bot

This comment has been minimized.

@gsmet
Copy link
Member

gsmet commented Dec 1, 2025

@mocenas it was a question :). But I think you answered it and it's all fine.

Basically I wanted to know if it was a reproducer for an issue to fix or an additional test. I understand it's an additional test.

@quarkus-bot

This comment has been minimized.

@mocenas mocenas force-pushed the kafka_test_donation branch from 44de84c to f2d10b2 Compare December 4, 2025 10:03
@mocenas
Copy link
Contributor Author

mocenas commented Dec 4, 2025

I decided to put this test into separate module in the end. When I tried to add it into existing integration-tests in just kept breaking stuff. E.g. The module I tried to put it in originally integration-tests/reactive-messaging-kafka turned out to be really sensitive about order of execution of testClasses. It was IMHO not worth to refactor the entire module or hack it to enforce order of execution.

@geoand
Copy link
Contributor

geoand commented Dec 5, 2025

Can you please update the title of the PR, as it currently requires someone looking at the issue to figure out even the basic context

@mocenas mocenas changed the title Reproducer for issue 50751 Reproducer for issue for double quotes in kafka companion channel names Dec 5, 2025
@quarkus-bot

This comment has been minimized.

@geoand
Copy link
Contributor

geoand commented Dec 5, 2025

I'll let @ozangunalp decide how he wants to handle this, but bear in mind a couple things:

  • We usually don't create new integration test modules unless we have to
  • I believe it would be better to use *Emitter instead of *Transmitter as it aligns with the Quarkus Messaging terminology

@ozangunalp
Copy link
Contributor

I agree that we can add a test like this. Probably we also need to add one with the usage of InMemoryConnector for tests. That's where we had the regression.

We can add this test to an existing test module with more descriptive bean & test names such as MessageProducer and UsingKafkaCompanionTest. Naming is hard :)

@mocenas mocenas force-pushed the kafka_test_donation branch from f2d10b2 to 2a1c612 Compare December 5, 2025 12:12
@mocenas
Copy link
Contributor Author

mocenas commented Dec 5, 2025

OK, I renamed the classes and moved the reproducer into integration-tests/reactive-messaging-kafka module.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants